-
-
Notifications
You must be signed in to change notification settings - Fork 187
fix(rsc/cjs): unwrap default
based on __cjs_module_runner_transform
marker
#905
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(rsc/cjs): unwrap default
based on __cjs_module_runner_transform
marker
#905
Conversation
- Add __cjs_interop__ runtime helper to handle ESM modules correctly - Check for __esModule flag and default export before accessing .default - Update all tests to reflect new transformation output - Fixes issue where require("esm-package") would fail when transformed Co-authored-by: hi-ogawa <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot What about #904 (comment)?
- Or we check during runtime
checkIfItShouldPickDefaultOrNot(await import("..."))
based on some heuristics.I think this works. Let's do:
- inject some marker export like
export const __cjs_module_runner_transform = true
for modules transformed bycjsModuleRunnerPlugin
.- then
checkIfItShouldPickDefaultOrNot
can unwrapdefault
only when__cjs_module_runner_transform
exists.
…rop helper - Change interop helper to check for __cjs_module_runner_transform marker - Add marker export to all transformed CJS modules - Update tests to include marker in transformed modules - More explicit approach than heuristic-based detection Co-authored-by: hi-ogawa <[email protected]>
Updated the implementation to use the explicit marker approach. The interop helper now checks for |
default
based on __cjs_module_runner_transform
marker
Problem
The
cjsModuleRunnerPlugin
was incorrectly transforming CommonJSrequire()
calls by always accessing.default
on the imported module:This assumption fails when Vite's import analysis resolves the import to an ESM variant of a package. For example, when importing
@swc/helpers/_/_interop_require_default
, Vite may resolve it to@swc/helpers/esm/_interop_require_default.js
. Pure ESM modules with only named exports don't have a.default
property, causing.default
to returnundefined
and breaking code that depends on these imports.This issue was discovered when trying to load Next.js CommonJS modules that import from packages like
@swc/helpers
, resulting in errors like:Solution
Introduced a runtime interop helper
__cjs_interop__
that uses an explicit marker-based approach to distinguish transformed CJS modules from genuine ESM modules:All modules transformed by
cjsModuleRunnerPlugin
now export an explicit marker:The helper only unwraps
.default
when the__cjs_module_runner_transform
marker is present, ensuring we don't incorrectly unwrap.default
on genuine ESM modules. This explicit approach is safer and more predictable than heuristic-based detection.Changes
transformCjsToEsm()
to inject the interop helper and wrap all dynamic imports__cjs_module_runner_transform
marker export to all transformed CJS modulesrequire()
callsrequire()
calls are present in the codeTesting
Fixes #904
Original prompt
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.